-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
2704 backward accesses #2814
2704 backward accesses #2814
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2814 +/- ##
========================================
Coverage 99.88% 99.88%
========================================
Files 359 359
Lines 50681 50833 +152
========================================
+ Hits 50625 50777 +152
Misses 56 56 ☔ View full report in Codecov by Sentry. |
…they weren't inside the search area, and added tests for previous_accesses in Reference class
@sergisiso This is ready for a first look now I think |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LonelyCat124 This is in a good shape, all the comments are minor. Some of the logic is quite complicated but since this is self-contained and there are a good amount of tests I am happy with it.
@sergisiso should be ready for another look now I think |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LonelyCat124 All changes have been addressed. This has a self-contained implementation with thorough tests, the remaining doubts about the functionality will be better addressed when we have practical examples of its usage.
I removed some leftover prints from the tests and update a couple of lines from the documentation to mention that backwards dependencies are now supported.
This is ready to merge.
Bugs fixed, initial implementation - need to add some other parts to it.